Antalya 26:6 Fix alter operations for iceberg - #2157
Conversation
|
ADD COLUMN DROP COLUMN RENAME COLUMN |
| { | ||
| switch (type->getTypeId()) | ||
| { | ||
| case TypeIndex::UInt8: |
There was a problem hiding this comment.
Added support for bool
| return {"string", true}; | ||
| case TypeIndex::UUID: | ||
| return {"uuid", true}; | ||
| case TypeIndex::Decimal32: |
There was a problem hiding this comment.
Added support for iceberg decimal types.
….com/Altinity/ClickHouse into antalya_26_6_fix_alter_table_iceberg
|
Decimal support |
|
Context: #2090 Retested with this PR's build. Previously this class of failure surfaced as Code 736 (Iceberg alter: catalog commit failed … after metadata file was written successfully). With the retry loop, permanent catalog rejections are retried until the budget is exhausted, so the user-visible code is now 290. Every retry hits the same HTTP 400: So ClickHouse is proposing a schema update whose last-column-id decreases. Iceberg requires last-column-id to be non-decreasing. On DROP COLUMN (and any schema evolution), is the last-column-id being written incorrectly somewhere? |
|
I asked Claude to analyze this PR and it came with some review. Please have a look: https://gist.github.com/mkmkme/b28a41f136a62d5a6d41a73dee0132bd |
….com/Altinity/ClickHouse into antalya_26_6_fix_alter_table_iceberg
….com/Altinity/ClickHouse into antalya_26_6_fix_alter_table_iceberg
… fix medium defects
|
@mkmkme can u check again please |
@subkanthi the new one is quite cleaner, but there're still some issues worth fixing: https://gist.github.com/mkmkme/98ffbceed6d86e14339537481a5f162c |
|
|
||
| } | ||
|
|
||
| MetadataGenerator::MetadataGenerator(Poco::JSON::Object::Ptr metadata_object_) |
There was a problem hiding this comment.
Just noting that dropping a column used in partitioning or ordering has had its behaviour changed, especially that of an ordering column which is now completely rejected.
There was a problem hiding this comment.
This is an iceberg spec:
(Section "Schema Evolution") explicitly forbids this: a column that is part of the current sort order or partition spec cannot be removed without first changing the sort order/partition spec to no longer reference it.
Thanks @mkmkme , i will take a shot at fixing 1, 2, for #5 Glue catalog we have separate issues which I thought is better to go in a separate PR. ALTER TABLE commit kept losing to concurrent modifications after 100 retries There was no concurrent modification. And the orphan v<N+1>.metadata.json makes every subsequent ALTER on that table repeat this until someone deletes it by hand. The loop structure is pre-existing; the misleading message is new, and the unused cleanup failpoint suggests the real fix was intended. |
|
|
Could you fix the conflicts please? |
continuation of work from #1841
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
...
Documentation entry for user-facing changes
...
CI/CD Options
Exclude tests:
Regression jobs to run: